Skip to content

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Sep 16, 2025

Q A
Type feature
BC Break yes
Fixed issues

Summary

This PR adds a new type to map Symfony UUID instances to a BSON Binary with subtype UUID. In addition, the existing AutoGenerator is replaced by separate ObjectIdGenerator and UUIDGenerator to support auto-generating new object IDs.

TODO:

  • Add new type for UUIDs
  • Refactor ID generation to allow generating UUIDs using the AUTO strategy.
  • Document new type and generator options

@alcaeus alcaeus force-pushed the support-symfony-uuid branch from f0c625b to 570bfff Compare September 17, 2025 18:00
@alcaeus alcaeus requested a review from Copilot September 17, 2025 18:10
@alcaeus alcaeus marked this pull request as ready for review September 17, 2025 18:10
@alcaeus alcaeus requested a review from GromNaN September 17, 2025 18:11
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces support for Symfony UUID components by adding a new UUID type and refactoring the ID generation strategy to support auto-generating UUIDs alongside ObjectIds.

  • Adds a new BinaryUuidType for mapping Symfony UUID instances to MongoDB Binary with UUID subtype
  • Refactors AUTO ID generation to support both ObjectId and UUID types through separate generators
  • Updates documentation to reflect the new UUID support and deprecates the old UUID generator

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
lib/Doctrine/ODM/MongoDB/Types/BinaryUuidType.php New type implementation for converting between Symfony UUIDs and MongoDB Binary
lib/Doctrine/ODM/MongoDB/Types/Type.php Adds UUID constant and type registration, updates type detection logic
lib/Doctrine/ODM/MongoDB/Id/SymfonyUuidGenerator.php New generator for creating Symfony UUIDs with support for v1, v4, and v7
lib/Doctrine/ODM/MongoDB/Id/ObjectIdGenerator.php New dedicated generator for ObjectIds, extracted from AutoGenerator
lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadataFactory.php Refactors AUTO generation logic to choose appropriate generator based on field type
tests/Doctrine/ODM/MongoDB/Tests/Types/BinaryUuidTypeTest.php Comprehensive test coverage for the new UUID type
tests/Doctrine/ODM/MongoDB/Tests/Functional/UuidMappingTest.php Functional tests for UUID field mapping and auto-generation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@alcaeus alcaeus requested a review from GromNaN September 23, 2025 10:07
@alcaeus alcaeus force-pushed the support-symfony-uuid branch from ce0a752 to 26d71ea Compare September 24, 2025 08:04
@alcaeus alcaeus requested a review from GromNaN September 24, 2025 08:04
throw new Exception(sprintf('Invalid binary data of type %d received for Uuid', $value->getType()));
}

return Uuid::fromBinary($value->getData());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked that this returns instances of UuidVx.
If there is a miss-match between the UUID version of the value in the database, and the document property type, the user will get a type error, which is perfect.


namespace Doctrine\ODM\MongoDB\Types;

use Exception;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
use Exception;
use Doctrine\ODM\MongoDB\MongoDBException;

}

if ($value->getType() !== Binary::TYPE_UUID) {
throw new Exception(sprintf('Invalid binary data of type %d received for Uuid', $value->getType()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a more specific exception make it catchable.

Suggested change
throw new Exception(sprintf('Invalid binary data of type %d received for Uuid', $value->getType()));
throw MongoDBException::invalidValueForType(sprintf('Invalid binary data of type %d received for Uuid', $value->getType()));

Comment on lines +2115 to +2126
-
message: '#^Call to static method PHPUnit\\Framework\\Assert\:\:assertSame\(\) with arguments MongoDB\\BSON\\Binary, null and ''Binary UUIDs are…'' will always evaluate to false\.$#'
identifier: staticMethod.impossibleType
count: 1
path: tests/Doctrine/ODM/MongoDB/Tests/Types/BinaryUuidTypeTest.php

-
message: '#^Call to static method PHPUnit\\Framework\\Assert\:\:assertSame\(\) with arguments Symfony\\Component\\Uid\\UuidV4, null and ''Uuid objects are…'' will always evaluate to false\.$#'
identifier: staticMethod.impossibleType
count: 1
path: tests/Doctrine/ODM/MongoDB/Tests/Types/BinaryUuidTypeTest.php

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which lines are this errors. Is it a false-positive due to the way the PHP extension can by-pass userland rules?

@GromNaN GromNaN added this to the 2.13.0 milestone Sep 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants